Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="internal/config/config/command.go" line_range="138-147" />
<code_context>

-func (c *Command) GetEnv(cfg Config) (map[string]string, error) {
-	if err := c.Env.Execute(cfg, cfg.GetEnv()); err != nil {
+func (c *Command) GetEnv(cfg Config, builtinEnv map[string]string) (map[string]string, error) {
+	if c.resolvedEnv != nil {
+		return cloneMap(c.resolvedEnv), nil
+	}
+
+	baseEnv := cloneMap(builtinEnv)
+	if baseEnv == nil {
+		baseEnv = make(map[string]string)
+	}
+	for key, value := range cfg.GetEnv() {
+		baseEnv[key] = value
+	}
+
+	if err := c.Env.Execute(cfg, baseEnv); err != nil {
 		return nil, err
 	}

-	return c.Env.Dump(), nil
+	filenameEnv := cloneMap(baseEnv)
+	for key, value := range c.Env.Dump() {
+		filenameEnv[key] = value
+	}
+
+	envFileEnv, err := c.EnvFiles.Load(cfg, filenameEnv)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve env_file for command '%s': %w", c.Name, err)
+	}
+
+	c.resolvedEnv = c.Env.Dump()
+	for key, value := range envFileEnv {
+		c.resolvedEnv[key] = value
+	}
+
+	return cloneMap(c.resolvedEnv), nil
 }

</code_context>
<issue_to_address>
**issue (bug_risk):** Command env caching ignores builtinEnv, so repeated runs with different args/dirs/shell reuse incorrect env

`c.resolvedEnv` is cached per `Command` instance, but the computed env depends on `builtinEnv` (`LETS_COMMAND_ARGS`, `LETS_COMMAND_WORK_DIR`, `LETS_SHELL`, etc.). Re-invoking the same `Command` with different args/dirs/shell will still return a clone of the first computed env, ignoring later `builtinEnv` values.

To avoid stale envs across invocations, either remove caching at the `Command` level or incorporate the relevant `builtinEnv` values into the cache key / `resolvedEnv` so each distinct invocation gets its own env.
</issue_to_address>

### Comment 2
<location path="internal/config/config/env_file.go" line_range="107-116" />
<code_context>
+func (e *EnvFiles) Load(cfg Config, envMap map[string]string) (map[string]string, error) {
</code_context>
<issue_to_address>
**issue (bug_risk):** EnvFiles.Load caches results without considering cfg or envMap, which can lead to incorrect reuse

The cache in `EnvFiles.Load` is keyed only by the `EnvFiles` instance, but the result also depends on `cfg.WorkDir` and `envMap` (via `expandWithEnv`). Reusing the same `EnvFiles` with different configs/envMaps will incorrectly return the first `loaded` result.

Either remove this caching, or scope it to a specific `(cfg, envMap)` context (or move caching to a higher layer where those inputs are stable).
</issue_to_address>

### Comment 3
<location path="internal/config/config/env_file.go" line_range="26-35" />
<code_context>
+	ready  bool
+}
+
+func (e *EnvFile) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var filename string
+	if err := unmarshal(&filename); err == nil {
+		e.Name = normalizeEnvFilename(filename)
+		e.Required = !isOptionalEnvFilename(filename)
+		if e.Name == "" {
+			return errors.New("env_file name can not be empty")
+		}
+
+		return nil
+	}
+
+	var raw struct {
+		Name     string
+		Required *bool
+	}
+	if err := unmarshal(&raw); err != nil {
+		return err
+	}
+
+	if raw.Name == "" {
+		return errors.New("env_file name can not be empty")
+	}
+
+	e.Name = raw.Name
+	e.Required = true
+	if raw.Required != nil {
+		e.Required = *raw.Required
+	}
+
+	return nil
+}
+
</code_context>
<issue_to_address>
**question:** Scalar and map env_file forms handle optional filenames differently; consider normalizing behavior

In the scalar form, `normalizeEnvFilename`/`isOptionalEnvFilename` treat a leading `-` as “optional” and strip it from the stored name. In the map form, `raw.Name` is used verbatim and `Required` defaults to true, so `name: -file.env` stays `-file.env` and is not treated as optional. If the map form is meant to mirror the scalar semantics, consider applying the same normalization/optionality logic to `raw.Name`.
</issue_to_address>

### Comment 4
<location path="internal/config/config/env_file_test.go" line_range="36" />
<code_context>
+	}
+}
+
+func TestParseEnvFiles(t *testing.T) {
+	t.Run("parses mixed env_file forms", func(t *testing.T) {
+		text := dedent.Dedent(`
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for scalar and map `env_file` forms and invalid top-level kinds

`TestParseEnvFiles` currently only exercises the sequence form and the error for a map item missing `name`. To improve coverage, please also add:

- A scalar case (`env_file: .env`) to confirm single-string values parse correctly.
- A valid map case (e.g. `env_file: { name: .env.prod, required: false }`) to verify `required` is respected.
- An invalid top-level kind (e.g. integer or boolean) to assert the `env_file must be a string, map, or sequence` error from `EnvFiles.UnmarshalYAML` is triggered.

Suggested implementation:

```golang
func TestParseEnvFiles(t *testing.T) {
	t.Run("parses scalar env_file", func(t *testing.T) {
		text := dedent.Dedent(`
		env_file: .env
		`)

		var raw struct {
			EnvFiles *EnvFiles `yaml:"env_file"`
		}

		if err := yaml.Unmarshal([]byte(text), &raw); err != nil {
			t.Fatalf("unexpected error unmarshalling scalar env_file: %v", err)
		}

		if raw.EnvFiles == nil {
			t.Fatalf("expected EnvFiles to be populated for scalar env_file")
		}

		if len(*raw.EnvFiles) != 1 {
			t.Fatalf("expected 1 env_file entry, got %d", len(*raw.EnvFiles))
		}

		// Adjust field names if they differ in EnvFile
		if (*raw.EnvFiles)[0].Name != ".env" {
			t.Fatalf("expected env_file name '.env', got %q", (*raw.EnvFiles)[0].Name)
		}
	})

	t.Run("parses map env_file", func(t *testing.T) {
		text := dedent.Dedent(`
		env_file:
		  name: .env.prod
		  required: false
		`)

		var raw struct {
			EnvFiles *EnvFiles `yaml:"env_file"`
		}

		if err := yaml.Unmarshal([]byte(text), &raw); err != nil {
			t.Fatalf("unexpected error unmarshalling map env_file: %v", err)
		}

		if raw.EnvFiles == nil {
			t.Fatalf("expected EnvFiles to be populated for map env_file")
		}

		if len(*raw.EnvFiles) != 1 {
			t.Fatalf("expected 1 env_file entry, got %d", len(*raw.EnvFiles))
		}

		ef := (*raw.EnvFiles)[0]

		// Adjust field names if they differ in EnvFile
		if ef.Name != ".env.prod" {
			t.Fatalf("expected env_file name '.env.prod', got %q", ef.Name)
		}

		if ef.Required {
			t.Fatalf("expected env_file required=false to be respected")
		}
	})

	t.Run("errors on invalid top-level kind", func(t *testing.T) {
		text := dedent.Dedent(`
		env_file: 123
		`)

		var raw struct {
			EnvFiles *EnvFiles `yaml:"env_file"`
		}

		err := yaml.Unmarshal([]byte(text), &raw)
		if err == nil {
			t.Fatalf("expected error unmarshalling invalid top-level env_file kind, got nil")
		}

		if !strings.Contains(err.Error(), "env_file must be a string, map, or sequence") {
			t.Fatalf("expected env_file kind error, got: %v", err)
		}
	})

	t.Run("parses mixed env_file forms", func(t *testing.T) {
		text := dedent.Dedent(`
		env_file:
		  - .env
		  - -.env.local
		  - name: .env.prod
		    required: false
		`)

		var raw struct {
			EnvFiles *EnvFiles `yaml:"env_file"`
		}

```

1. Ensure `yaml` and `strings` are imported in this test file (they likely already are for `yaml`, but `strings` may need to be added):
   - `gopkg.in/yaml.v3` (or whatever YAML package the file already uses).
   - `"strings"` for the error substring check.
2. The code above assumes:
   - `EnvFiles` is a slice-like type (`[]EnvFile`) so that `len(*raw.EnvFiles)` and index access are valid.
   - The element type has fields `Name string` and `Required bool`.
   If the actual type or field names differ (e.g. `Path` instead of `Name`, or `Optional` instead of `Required`), update the field accesses and assertions accordingly.
3. Keep the remainder of the existing `"parses mixed env_file forms"` subtest body (assertions, error checks, etc.) after the shown snippet so current coverage is preserved.
</issue_to_address>

### Comment 5
<location path="internal/config/config/env_file_test.go" line_range="90" />
<code_context>
+	})
+}
+
+func TestEnvFilesLoad(t *testing.T) {
+	workDir := t.TempDir()
+	writeFixtureFile(t, workDir, ".env.first", "VALUE=first\n")
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test to cover filename interpolation precedence between provided env map and process environment

Current `TestEnvFilesLoad` cases only pass `nil` for the env map, so they don’t cover precedence between the explicit env map and `os.Environ()` during filename interpolation. Please add a subtest that:

- Sets `os.Setenv("TARGET", "from-os")`.
- Calls `Load` with an env map containing `TARGET=from-map` and a pattern like `.env.${TARGET}`.
- Asserts the selected filename resolves using `from-map`, not `from-os`.

This will lock in the intended precedence behavior for filename expansion.

Suggested implementation:

```golang
	cfg := Config{WorkDir: workDir}

	t.Run("env map overrides process env for filename interpolation", func(t *testing.T) {
		t.Setenv("TARGET", "from-os")

		envFiles := &EnvFiles{
			Items: []EnvFile{
				{Name: ".env.${TARGET}", Required: true},
			},
		}

		env := map[string]string{
			"TARGET": "from-map",
		}

		ctx := context.Background()
		values, err := cfg.LoadEnvFiles(ctx, envFiles, env)
		if err != nil {
			t.Fatalf("LoadEnvFiles failed: %v", err)
		}

		if got, want := values["VALUE"], "first"; got != want {
			t.Fatalf("unexpected VALUE: got %q, want %q", got, want)
		}
	})

	t.Run("later files override earlier files", func(t *testing.T) {

```

1. Ensure `context` is imported at the top of `env_file_test.go`:
   - If not present, add `import "context"` (or include it in the existing import block).
2. If the function under test has a different name or signature than `cfg.LoadEnvFiles(ctx, envFiles, env)`, adjust the call accordingly so that:
   - The first argument is a `context.Context` (or whatever your existing tests use).
   - The second argument is the `EnvFiles` collection.
   - The third argument is the explicit env map used for interpolation.
3. The assertion assumes `.env.first` contains `VALUE=first` and that `.env.${TARGET}` is resolved using `TARGET` from the provided env map; if your API returns values in a different structure, adapt the indexing (`values["VALUE"]`) to your actual return type.
</issue_to_address>

### Comment 6
<location path="internal/config/config/env_file_test.go" line_range="165" />
<code_context>
+	})
+}
+
+func TestConfigSetupEnvWithEnvFile(t *testing.T) {
+	workDir := t.TempDir()
+	writeFixtureFile(t, workDir, ".env.global", "FROM_FILE=global-file\nOVERRIDE=from-file\n")
</code_context>
<issue_to_address>
**suggestion (testing):** Add an assertion that subsequent GetEnv calls are stable and do not mutate shared state

Since config env is cached, consider also asserting that a second `cfg.GetEnv()` call returns an equal but distinct map (mutating the first result does not affect the second). This clarifies the contract that callers get an isolated copy while still benefiting from caching.

Suggested implementation:

```golang
package config

import (
	"testing"

	"github.com/lithammer/dedent"
)

// TestGetEnvReturnsIsolatedCopies verifies that cfg.GetEnv() is stable
// and returns an equal but distinct map on subsequent calls, so that
// mutating the first result does not affect the second or subsequent calls.
func TestGetEnvReturnsIsolatedCopies(t *testing.T) {
	workDir := t.TempDir()

	cfg := loadConfigFixture(t, workDir, dedent.Dedent(`
	shell: bash
	env:
	  FOO: foo
	  BAR: bar
	`))

	// First call to GetEnv
	env1 := cfg.GetEnv()
	if env1["FOO"] != "foo" || env1["BAR"] != "bar" {
		t.Fatalf("unexpected env1 contents: %#v", env1)
	}

	// Second call should return the same logical values
	env2 := cfg.GetEnv()
	if env2["FOO"] != "foo" || env2["BAR"] != "bar" {
		t.Fatalf("unexpected env2 contents: %#v", env2)
	}

	// The maps should be logically equal
	if len(env1) != len(env2) {
		t.Fatalf("env1 and env2 length differ: %d vs %d", len(env1), len(env2))
	}
	for k, v1 := range env1 {
		if v2, ok := env2[k]; !ok || v1 != v2 {
			t.Fatalf("env mismatch for key %q: env1=%q env2=%q (present=%v)", k, v1, v2, ok)
		}
	}

	// Mutate the first map and ensure it does not affect the second
	env1["NEW_FROM_ENV1"] = "value"
	delete(env1, "FOO")

	if _, ok := env2["NEW_FROM_ENV1"]; ok {
		t.Fatalf("env2 unexpectedly saw mutation from env1: %#v", env2)
	}
	if env2["FOO"] != "foo" {
		t.Fatalf("env2[\"FOO\"] changed after mutating env1: %#v", env2)
	}

	// A third call should also be unaffected by mutations to env1/env2
	env3 := cfg.GetEnv()
	if env3["FOO"] != "foo" || env3["BAR"] != "bar" {
		t.Fatalf("env3 contents unexpectedly changed: %#v", env3)
	}
	if _, ok := env3["NEW_FROM_ENV1"]; ok {
		t.Fatalf("env3 unexpectedly saw mutation from env1: %#v", env3)
	}
}

```

1. Ensure the package name `package config` at the top of the new file matches the existing package declaration in `internal/config/config/env_file_test.go` and other tests in this directory; adjust it if the package name is different.
2. This test assumes that `loadConfigFixture` and `writeFixtureFile` (if needed elsewhere) are helper functions already defined in the same package, as in the existing tests; if they are in a different package or have different names, update the calls accordingly.
3. If `github.com/lithammer/dedent` is imported in a common `_test.go` file under the same package, you can remove the explicit import here and rely on that, or adjust the import path if your project uses a local dedent helper instead.
</issue_to_address>